fix(skia): Keep Win32 UIA providers alive on removal by morning4coffe-dev · Pull Request #23613 · unoplatform/uno

morning4coffe-dev · 2026-07-01T10:55:51Z

Fixes #494

GitHub Issue: closes https://github.com/unoplatform/kahua-private/issues/494

PR Type:

🐞 Bugfix

What changed? 🚀

PR Checklist ✅

🧪 Added Runtime tests, UI tests, or a manual test sample (for bug fixes / features, if applicable)
📚 Docs have been added/updated following the documentation template (for bug fixes / features)
🖼️ Validated PR Screenshots Compare Test Run results.
❗ Contains NO breaking changes
👀 Reviewed 2 other open pull requests (optional but appreciated!)

Fixes NoClickablePointException / 0x80070002 (FileNotFound) raised to out-of-proc UIA clients (FlaUI, WinAppDriver, Narrator) when calling into an element whose subtree was removed. - Port CUIAWrapper::Invalidate: providers now expose Invalidate(), which disconnects from UIA, cascades to virtual (DataGrid) children, and makes every entry point return UIA_E_ELEMENTNOTAVAILABLE via ThrowIfDisconnected, instead of the COM-callable wrapper being GC'd out from under a cached client proxy. A short-lived strong tombstone keeps the wrapper alive until UIA processes the disconnect. - Wire UIA_ClickablePointPropertyId to AutomationPeer.GetClickablePoint, matching CUIAWrapper (VT_EMPTY when (0,0), else client-to-screen). Fixes #494

github-actions · 2026-07-01T10:56:10Z

Claude finished @morning4coffe-dev's task in 5m 59s —— View job

PR Review

Gather context and read changed files
Review Win32Accessibility.cs
Review Win32RawElementProvider.cs
Review Win32UIAutomationInterop.cs
Post inline comments for findings
Post summary

Summary

Verdict: ship with nits — 1 low finding (consistency gap), 0 blocking issues.

The core fix is correct and well-designed. The tombstone set (_disconnectedProviders) correctly addresses the CCW (COM-callable wrapper) lifetime race: without it, a provider's CCW could be GC'd in the window between UiaDisconnectProvider and UIA delivering the structure-changed notification to out-of-proc clients, causing 0x80070002 instead of the expected UIA_E_ELEMENTNOTAVAILABLE. The ordering is correct — providers are rooted in the tombstone before UiaDisconnectProvider is called, and drained only after UiaRaiseStructureChangedEvent completes. The Invalidate() cascade to cached children correctly extends the fix to virtual peers (e.g. WCT DataGrid rows) that are not reachable via the element table.

Findings

1 Low — COM API contract inconsistency (inline comment posted)

ThrowIfDisconnected() was added to 9 COM interface methods, but 3 are still unguarded: HostRawElementProvider, GetEmbeddedFragmentRoots, and FragmentRoot. Harmless in practice (explained in the inline comment), but breaks the stated invariant.

Other observations

GetClickablePoint (0,0) → null: Correctly matches WinUI parity (GetClickablePointCore default returns (0,0)), and documented in comments. Elements literally positioned at the window's top-left origin will not surface a clickable point — this is an accepted known limitation matching WinUI.
No runtime test coverage: The checklist correctly flags this as not done. Given this is a COM lifetime fix for screen readers, a manual test sample (e.g. using FlaUI or Inspect.exe to verify UIA_E_ELEMENTNOTAVAILABLE is returned after element removal) would be valuable for regression prevention. Consider adding a note in the issue about manual verification performed.
Comment length on Invalidate(): The <remarks> block is longer than the "a line or two" default per code style, but the CCW lifetime invariant is genuinely complex and the MUX-reference porting context justifies it.

Copilot

Pull request overview

This PR adjusts the Win32 (Skia) UI Automation (UIA) provider lifecycle so that providers are explicitly disconnected/invalidated when their subtree is removed, and are kept alive briefly to ensure out-of-proc UIA clients observe the expected UIA_E_ELEMENTNOTAVAILABLE behavior rather than failures caused by prematurely collected COM-callable wrappers.

Changes:

Add an explicit provider invalidation path (Win32RawElementProvider.Invalidate) that disconnects from UIA and makes subsequent calls fail with UIA_E_ELEMENTNOTAVAILABLE.
Add “tombstoning” in Win32Accessibility to keep just-disconnected providers strongly referenced until the coalesced structure-changed flush completes.
Expose UIA_ClickablePointPropertyId via GetPropertyValue (with coordinate conversion) and introduce the UIA_E_ELEMENTNOTAVAILABLE HRESULT constant.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32UIAutomationInterop.cs	Adds the `UIA_E_ELEMENTNOTAVAILABLE` HRESULT constant for consistent COM error reporting after disconnect.
src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32RawElementProvider.cs	Implements provider invalidation + disconnection behavior and expands property/pattern surface (incl. ClickablePoint).
src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32Accessibility.cs	Keeps disconnected providers alive until structure-change notifications flush, preventing stale proxies from hitting severed CCWs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

unodevops · 2026-07-01T11:22:49Z

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-23613/wasm-skia-net9/index.html

unodevops · 2026-07-01T12:35:23Z

⚠️⚠️ The build 220387 has failed on Uno.UI - CI.

github-actions · 2026-07-04T12:31:43Z

Claude finished @morning4coffe-dev's task in 3m 44s —— View job

PR Review

Verdict: ship — all blocking findings from the prior review are resolved; 1 low nit (inline comment posted).

What changed since last review

Commit 364b1443 added the missing ThrowIfDisconnected() calls to the three previously unguarded COM interface members (HostRawElementProvider, GetEmbeddedFragmentRoots(), FragmentRoot). The invariant — that every subsequent call on an invalidated provider raises UIA_E_ELEMENTNOTAVAILABLE — is now consistently enforced across all interface entry points.

Core fix — confirmed correct

The tombstone/Invalidate design is sound:

Ordering in Invalidate(): _isInvalidated = true → cascade peer-tree children → OnProviderInvalidated (roots provider in _disconnectedProviders, removes from lookup tables) → UiaDisconnectProvider. The CCW is in the tombstone before the disconnect is issued, so no GC window exists.
Cascade via _cachedAutomationChildren: catches virtual peers (WCT DataGrid rows/cells) that are keyed only by peer and are invisible to the visual-tree walk in CleanupProviders. The cycle guard (visited set + _isInvalidated early-return) prevents double-disconnection.
Tombstone drain timing: _disconnectedProviders.Clear() happens only after UiaRaiseStructureChangedEvent completes for all pending providers, so the CCW outlives the entire structure-change notification window.
Virtual-provider safety: OnProviderInvalidated guards the _providers eviction with ReferenceEquals(byElement, provider), so a virtual provider that shares its owner UIElement with the canonical peer cannot evict the canonical peer's entry.

Other observations

GetClickablePoint (0,0) sentinel: matches WinUI parity per the MUX reference; documented in the inline comment. Elements at the top-left origin lose their explicit clickable point, which is acceptable and consistent with the reference platform.
No runtime test coverage: the checklist item remains unchecked. Given this is a CCW-lifetime fix that requires an out-of-proc UIA client (Narrator, FlaUI, Inspect.exe), a manual verification note in the issue would be valuable for future regressions.
1 low nit (inline comment): _pendingStructureChanges should use ReferenceEqualityComparer.Instance for consistency with _disconnectedProviders; functionally equivalent, just style.

github-actions · 2026-07-04T12:35:20Z

+	// delivering the structure-changed notification, so an out-of-proc client
+	// that still holds the proxy observes UIA_E_ELEMENTNOTAVAILABLE rather than a
+	// severed-CCW 0x80070002. Drained once the structure-change flush completes.
+	private readonly HashSet<Win32RawElementProvider> _disconnectedProviders = new(ReferenceEqualityComparer.Instance);


_pendingStructureChanges (declared just above) holds the same reference-typed Win32RawElementProvider values but is constructed without an explicit comparer, while this field spells out ReferenceEqualityComparer.Instance. Since Win32RawElementProvider doesn't override Equals/GetHashCode the behaviour is identical, but for clarity and consistency it's worth aligning them:

Suggested change

private readonly HashSet<Win32RawElementProvider> _disconnectedProviders = new(ReferenceEqualityComparer.Instance);

private readonly HashSet<Win32RawElementProvider> _pendingStructureChanges = new(ReferenceEqualityComparer.Instance);

// Strong references to just-invalidated providers. Keeps their COM-callable

// wrappers alive across the window between UiaDisconnectProvider and UIA

// delivering the structure-changed notification, so an out-of-proc client

// that still holds the proxy observes UIA_E_ELEMENTNOTAVAILABLE rather than a

// severed-CCW 0x80070002. Drained once the structure-change flush completes.

private readonly HashSet<Win32RawElementProvider> _disconnectedProviders = new(ReferenceEqualityComparer.Instance);

unodevops · 2026-07-04T12:53:40Z

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-23613/wasm-skia-net9/index.html

unodevops · 2026-07-04T13:55:15Z

⚠️⚠️ The build 220715 has failed on Uno.UI - CI.

Copilot AI review requested due to automatic review settings July 1, 2026 10:55

github-actions Bot assigned morning4coffe-dev Jul 1, 2026

Copilot started reviewing on behalf of morning4coffe-dev July 1, 2026 10:56 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32RawElementProvider.cs Outdated

Comment thread src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32RawElementProvider.cs

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Uno.UI.Runtime.Skia.Win32/Accessibility/Win32RawElementProvider.cs

fix: Ensure disconnection checks in Win32RawElementProvider methods

364b144

github-actions Bot reviewed Jul 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(skia): Keep Win32 UIA providers alive on removal#23613

fix(skia): Keep Win32 UIA providers alive on removal#23613
morning4coffe-dev wants to merge 2 commits into
masterfrom
dev/doti/uia-noclickablepoint

morning4coffe-dev commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

unodevops commented Jul 1, 2026

Uh oh!

unodevops commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 4, 2026 •

edited

Loading

Uh oh!

github-actions Bot Jul 4, 2026

Uh oh!

unodevops commented Jul 4, 2026

Uh oh!

unodevops commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

morning4coffe-dev commented Jul 1, 2026

PR Type:

What changed? 🚀

PR Checklist ✅

Uh oh!

github-actions Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Summary

Findings

Other observations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

unodevops commented Jul 1, 2026

Uh oh!

unodevops commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

What changed since last review

Core fix — confirmed correct

Other observations

Uh oh!

github-actions Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

unodevops commented Jul 4, 2026

Uh oh!

unodevops commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 1, 2026 •

edited

Loading

github-actions Bot commented Jul 4, 2026 •

edited

Loading